Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

reflect state of started pipeline/task when using --showlog #1880

Closed
wants to merge 1 commit into from
Closed

reflect state of started pipeline/task when using --showlog #1880

wants to merge 1 commit into from

Conversation

avinal
Copy link
Member

@avinal avinal commented Feb 2, 2023

Signed-off-by: Avinal Kumar [email protected]

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes tests (if functionality changed/added)
  • Run the code checkers with make check
  • Regenerate the manpages, docs and go formatting with make generated
  • Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

using --showlog flag with tkn pipeline/task start now returns correct value based on final state of started pipelinerun/taskrun

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 2, 2023
@tekton-robot
Copy link
Contributor

Hi @avinal. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign vdemeester after the PR has been reviewed.
You can assign the PR to them by writing /assign @vdemeester in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 2, 2023
@vinamra28
Copy link
Member

/ok-to-test

@tekton-robot tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 2, 2023
@avinal
Copy link
Member Author

avinal commented Feb 2, 2023

Will fix the test cases tomorrow.

@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 22, 2023
@vinamra28
Copy link
Member

@avinal still working on this or can we close this for now and probably you can create a fresh PR once you get time to work on this?

@avinal
Copy link
Member Author

avinal commented Mar 27, 2023

Hi @vinamra28 yes, I am working, I need help with detecting if a PR/TR has failed. I referred to the documents, https://github.com/tektoncd/pipeline/blob/main/docs/pipelineruns.md#pipelinerun-status, is status Failed, a definitive way of knowing if our PR/TR has failed? I tried a couple of other tricks, but it gives ambiguous results.

@tekton-robot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 25, 2023
@anetbnd
Copy link

anetbnd commented Jun 26, 2023

/remove-lifecycle stale Please don't stop working on that topic. It is very important how otherwise it would be possible to trigger a pipeline from another pipeline and pass the result back the first one?

@tekton-robot
Copy link
Contributor

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 26, 2023
@anetbnd
Copy link

anetbnd commented Jul 26, 2023

/remove-lifecycle rotten Please don't forget this PR. It is an absolutely mandatory feature to trigger tkn pipelines from another pipeline (for example github actions).

@piyush-garg
Copy link
Contributor

/remove-lifecycle rotten

@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 16, 2023
@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 6, 2023
@avinal
reflect state of started pipeline/task when using --showlog
e30943e 
- this change fixes an issue where using --showlog with tkn start pipeline/task does not return a non-zero value when  created taskrun/pipelinerun fails.
- refer tektoncd#1820

Signed-off-by: Avinal Kumar <[email protected]>
@tekton-robot
Copy link
Contributor

@avinal: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-tekton-cli-integration-tests e30943e link true /test pull-tekton-cli-integration-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@piyush-garg piyush-garg mentioned this pull request Oct 25, 2023
Closed
4 tasks
@avinal
Copy link
Member Author

avinal commented Feb 27, 2024

Will open a new PR

@avinal avinal closed this Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pradeepitm12 pradeepitm12 Awaiting requested review from pradeepitm12

@vinamra28 vinamra28 Awaiting requested review from vinamra28

Assignees
No one assigned
Labels
ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Return value of tkn pipeline start with --showlog does not depend on state of started pipeline
5 participants
@avinal @tekton-robot @vinamra28 @anetbnd @piyush-garg